Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for the crash from #6367 #6375

Merged
merged 2 commits into from
Nov 2, 2021
Merged

Fix for the crash from #6367 #6375

merged 2 commits into from
Nov 2, 2021

Conversation

vksnk
Copy link
Member

@vksnk vksnk commented Nov 2, 2021

It would add an empty box for the function even if box_provided didn't return anything for this function, which would later cause a crash when we try to generate min/max lets for the given function.

@@ -1133,7 +1133,9 @@ class BoundsInference : public IRMutator {
auto boxes = boxes_provided(body, empty_scope, func_bounds);
for (const auto &fused : fused_group) {
string fused_stage_name = fused.first + ".s" + std::to_string(fused.second);
boxes_for_fused_group[fused_stage_name] = boxes[fused.first];
if (boxes.count(fused.first) > 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this do a double-lookup? How about:

auto it = boxes.find(fused.first);
if (it != boxes.end()) {
  boxes_for_fused_group[fused_stage_name] = it->second;
}

@@ -2110,6 +2110,93 @@ int rvar_bounds_test() {
return 0;
}

int two_compute_at_test() {
ImageParam input1(Int(16), 2, "input1");
;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stray semicolon

@@ -2110,6 +2110,93 @@ int rvar_bounds_test() {
return 0;
}

int two_compute_at_test() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth commenting with the issue number that this test case came from, so that later readers have context

@vksnk
Copy link
Member Author

vksnk commented Nov 2, 2021

Test failures are not related, going to merge.

@vksnk vksnk merged commit 5b8f473 into master Nov 2, 2021
@abadams abadams added the backport me This change should be backported to release versions label Jan 20, 2022
abadams pushed a commit that referenced this pull request Jan 20, 2022
* Skip empty boxes

* Address the comments

(cherry picked from commit 5b8f473)
abadams pushed a commit that referenced this pull request Jan 20, 2022
* Skip empty boxes

* Address the comments

(cherry picked from commit 5b8f473)
abadams pushed a commit that referenced this pull request Jan 20, 2022
* Skip empty boxes

* Address the comments

(cherry picked from commit 5b8f473)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport me This change should be backported to release versions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants